Skip to content

feat(artists): mark as favorite #90

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat(artists): mark as favorite #90

wants to merge 6 commits into from
+156 −21

Conversation

@KayBeSee
Copy link

@KayBeSee KayBeSee commented Jul 22, 2025 •
edited
Loading

closes #77

KayBeSee
KayBeSee commented Jul 22, 2025
View reviewed changes
src/hooks/useFavoriteState.ts Outdated
export function useFavoriteState(initialFavorites: string[]) {
const router = useRouter();
const defaultValue = initialFavorites ? JSON.stringify(initialFavorites) : '[]';
const [cookieValue, setCookieValue] = useCookie('relisten_favorites:artists', defaultValue);
Copy link
Author

@KayBeSee KayBeSee Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a really hard time DRYing the relisten_favorites:artists variable to reuse in src/lib/serverFavoriteCookies.ts. It kept saying the export was a function?

Also open to changing naming convention. Figure it was worth scoping to artist so a different cookie name can be used for tracks.

switz
switz reviewed Jul 22, 2025
View reviewed changes
src/lib/serverFavoriteCookies.ts Outdated
const cookieStore = await cookies();

try {
const value = cookieStore.get('relisten_favorites:artists')?.value;
Copy link
Member

@switz switz Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you export the cookie key so its declared once and imported?

Copy link
Author

@KayBeSee KayBeSee Jul 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. I added to src/lib/constants.ts b/c was getting some weird behavior importing/exporting between client and server components.

@KayBeSee KayBeSee requested a review from switz July 22, 2025 17:13
@switz
Copy link
Member

switz commented Jul 24, 2025

Is there any way we can make this work without calling router.refresh?

@switz
Copy link
Member

switz commented Jul 24, 2025

Also might be better to use a Set here:

  const toggleFavorite = useCallback(
    (artistId: string) => {
      const updatedFavorites = new Set([...parsedCookies]);

      if (parsedCookies.includes(artistId)) {
        updatedFavorites.delete(artistId);
      } else {
        updatedFavorites.add(artistId);
      }

      setFavorites([...updatedFavorites]);
    },
    [parsedCookies, setFavorites]
  );

@KayBeSee
Copy link
Author

KayBeSee commented Aug 5, 2025

Also might be better to use a Set here:

  const toggleFavorite = useCallback(
    (artistId: string) => {
      const updatedFavorites = new Set([...parsedCookies]);

      if (parsedCookies.includes(artistId)) {
        updatedFavorites.delete(artistId);
      } else {
        updatedFavorites.add(artistId);
      }

      setFavorites([...updatedFavorites]);
    },
    [parsedCookies, setFavorites]
  );

Good call, just updated.

Is there any way we can make this work without calling router.refresh?

I can mess around with it for sure. I was doing router.refresh based on this comment in the original issue.

@switz
Copy link
Member

switz commented Aug 7, 2025

yeah that's my bad, didn't think to realize it would also refetch the random shows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@switz switz Awaiting requested review from switz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Mark artist as favorite

2 participants

@KayBeSee @switz